Fix license cmd #2351
Fix license cmd #2351
Conversation
lib/control/control-commands.c
Outdated
| (GCompareFunc)control_command_start_with_command); | ||
| if (!command_it) | ||
| { | ||
| msg_error("Failed to replace control command", |
There was a problem hiding this comment.
hmm... I think we could register the command when we want to replace a non-existent (and print a warning msg): I'll change it.
|
Build SUCCESS |
1 similar comment
|
Build SUCCESS |
| { | ||
| msg_warning("Trying to replace a non-existent command. Command will be registered as a new command.", | ||
| evt_tag_str("command", command_name)); | ||
| control_register_command(command_name, description, function, user_data); |
There was a problem hiding this comment.
Do we need the warning here?
If we do, I think we need a bit more context, like mentioning that this was a control socket command, but I like the idea that modules can register such stuff.
But all in all I would make this msg_debug().
There was a problem hiding this comment.
From security point if view, I would sleep better if modules cannot change behavior of existing commands. I would probably have deleted the option if it is not needed, and allow modules to register their own commands as new commands. Other than that, the changeset looks good to me.
|
I think nothing protects from re-registering a command with We should rethink the |
|
@gaborznagy : fixed |
lbudai
force-pushed
the
fix-license-cmd
branch
from
4e32b53
to
47bf99c
Compare
|
Build SUCCESS |
Signed-off-by: Laszlo Budai <stentor.bgyk@gmail.com>
Signed-off-by: Laszlo Budai <stentor.bgyk@gmail.com>
…ent one Signed-off-by: Laszlo Budai <stentor.bgyk@gmail.com>
Signed-off-by: Laszlo Budai <stentor.bgyk@gmail.com>
lbudai
force-pushed
the
fix-license-cmd
branch
from
47bf99c
to
3af82c9
Compare
|
@furiel In what way would malicious modules be a threat vector? Because I
fail to see one. Just imagine that a root user (who is the only one
permitted to install/change modules) can already ptrace the process.
…On Fri, Oct 12, 2018, 09:01 furiel ***@***.***> wrote:
***@***.**** approved this pull request.
From security point if view, I would sleep better if modules cannot change
behavior of existing commands. I would probably have deleted the option if
it is not needed, and allow modules to register their own commands as new
commands. Other than that, the changeset looks good to me.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2351 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AArldvt4Bb_GtZkgBX6i-m4XSBFVYU47ks5ukD5kgaJpZM4XX00K>
.
|
|
Build SUCCESS |
@czanik : FYI